feat: share security contexts globally by roderik · Pull Request #30 · settlemint/network-bootstrapper

roderik · 2025-09-18T14:01:30Z

Summary

add global.securityContexts defaults for pod/container security context inheritance
merge global + chart-level security contexts when rendering bootstrapper and node workloads
refresh chart docs to reflect the new global configuration knobs

Testing

bun run check
bun run typecheck
bun run test

sourcery-ai

Sorry @roderik, you have reached your weekly rate limit of 1500000 diff characters.

Please try again later or upgrade to continue using Sourcery

gemini-code-assist

Summary of Changes

Hello @roderik, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the Helm chart's configuration capabilities by introducing global security contexts for Kubernetes pods and containers. This change centralizes the management of security settings, allowing users to define common security policies at a higher level that are then inherited by subcharts. It ensures that security contexts are consistently applied across different components of the deployed network, while still providing the flexibility for individual subcharts to specify their own security settings, which will be merged with the global defaults.

Highlights

Global Security Contexts: Introduced global.securityContexts to the main Helm chart, allowing for centralized definition of pod and container security contexts.
Security Context Inheritance: Implemented logic in subcharts (network-bootstrapper and network-nodes) to merge global security contexts with chart-specific overrides, ensuring consistent application while maintaining flexibility.
Documentation Updates: Updated README.md files for the main chart and subcharts to reflect the new global security context configuration and inheritance behavior.
Persistence Default Correction: Corrected a default value for persistence.enabled in the network-nodes subchart's README.md from false to true.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

github-actions · 2025-09-18T14:01:59Z

To view in Slack, search for: 1758204120.092509

gemini-code-assist

Code Review

This pull request introduces a valuable feature by adding global security contexts, allowing for centralized management of pod and container security settings across the Helm chart. The implementation correctly merges global and local contexts, with local settings taking precedence. The documentation has also been updated to reflect these new values.

I have a few suggestions for minor code cleanup in the Helm templates to remove some redundant code. Additionally, I've noted a change to the default value of persistence.enabled in the network-nodes subchart, which seems unrelated to this PR's main goal and could be a breaking change for users. It would be good to clarify if this change is intentional.

gemini-code-assist · 2025-09-18T14:02:54Z

 | persistence.accessModes | list | `["ReadWriteOnce"]` | Requested access modes for the PersistentVolumeClaim. |
 | persistence.annotations | object | `{}` |  |
-| persistence.enabled | bool | `false` | Enable persistent volume claims for ledger data. |
+| persistence.enabled | bool | `true` | Enable persistent volume claims for ledger data. |


The default value for persistence.enabled has been changed from false to true. This is a significant change in the chart's default behavior and could be a breaking change for users who rely on the previous default. This change also seems unrelated to the main purpose of this pull request, which is about global security contexts. Please confirm if this change is intentional. If it is, it should be highlighted in the pull request summary as a notable change.

gemini-code-assist · 2025-09-18T14:02:55Z

+      {{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) | default (dict) }}
+      {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) | default (dict) }}


The use of | default (dict) at the end of the merge pipelines is redundant. The merge function will always return a map, even if its inputs are empty maps. The dig and default functions used as inputs to merge already ensure that maps are being passed. Removing the final | default (dict) will make the code slightly cleaner. This applies to both $podSecurityContext and $containerSecurityContext variables.

{{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) }} {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) }}

gemini-code-assist · 2025-09-18T14:02:55Z

+  {{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) | default (dict) }}
+  {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) | default (dict) }}


The use of | default (dict) at the end of the merge pipelines is redundant. The merge function will always return a map. The dig and default functions used as inputs to merge already ensure that maps are being passed. Removing the final | default (dict) will make the code slightly cleaner. This applies to both $podSecurityContext and $containerSecurityContext variables.

{{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) }} {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) }}

gemini-code-assist · 2025-09-18T14:02:55Z

+  {{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) | default (dict) }}
+  {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) | default (dict) }}


The use of | default (dict) at the end of the merge pipelines is redundant. The merge function will always return a map. The dig and default functions used as inputs to merge already ensure that maps are being passed. Removing the final | default (dict) will make the code slightly cleaner. This applies to both $podSecurityContext and $containerSecurityContext variables.

{{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) }} {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) }}

seer-by-sentry · 2025-09-18T14:07:30Z

+      {{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) | default (dict) }}
+      {{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) | default (dict) }}


Potential bug: The arguments to the Helm merge function are reversed, causing global security contexts to incorrectly override local ones, contrary to the documented intent.

Description: The Helm merge function gives precedence to its first argument (the destination). In the current implementation, the global security context is passed as the first argument and the local .Values.podSecurityContext is the second. This causes global settings to always override local, chart-specific settings. The intended behavior, as indicated by comments in values.yaml, is for local values to take precedence over global ones. This logical error will cause user-provided local security contexts to be silently ignored whenever a global context is also defined, leading to incorrect configurations being applied without any warning.

Suggested fix: Reverse the order of the arguments in the merge function call. The local context, (default (dict) .Values.podSecurityContext), should be the first argument, and the global context, (deepCopy (dig "pod" $globalSecurityContexts (dict))), should be the second. This ensures local values correctly override global defaults.
_{severity: 0.65, confidence: 0.98}

_{Did we get this right? 👍 / 👎 to inform future reviews.}

feat(charts): share security contexts globally

be8d1f2

sourcery-ai Bot reviewed Sep 18, 2025

View reviewed changes

gemini-code-assist Bot reviewed Sep 18, 2025

View reviewed changes

github-actions Bot added the qa:running QA workflow is currently running label Sep 18, 2025

github-actions Bot added the status:ready-for-review Pull request is ready for review label Sep 18, 2025

gemini-code-assist Bot reviewed Sep 18, 2025

View reviewed changes

github-actions Bot added qa:success QA workflow passed successfully feat New feature and removed qa:running QA workflow is currently running labels Sep 18, 2025

seer-by-sentry Bot reviewed Sep 18, 2025

View reviewed changes

refactor(charts): drop redundant defaults

692be2b

github-actions Bot added qa:running QA workflow is currently running qa:success QA workflow passed successfully and removed qa:success QA workflow passed successfully qa:running QA workflow is currently running labels Sep 18, 2025

fix(charts): ensure local security contexts take precedence

6cffe65

github-actions Bot added qa:running QA workflow is currently running and removed qa:success QA workflow passed successfully labels Sep 18, 2025

roderik merged commit af4b626 into main Sep 18, 2025
7 checks passed

roderik deleted the feat/global-security-contexts branch September 18, 2025 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: share security contexts globally#30

feat: share security contexts globally#30
roderik merged 3 commits intomainfrom
feat/global-security-contexts

roderik commented Sep 18, 2025

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

github-actions Bot commented Sep 18, 2025 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Sep 18, 2025

Uh oh!

gemini-code-assist Bot Sep 18, 2025

Uh oh!

gemini-code-assist Bot Sep 18, 2025

Uh oh!

gemini-code-assist Bot Sep 18, 2025

Uh oh!

seer-by-sentry Bot Sep 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		{{- $podSecurityContext := merge (deepCopy (dig "pod" $globalSecurityContexts (dict))) (default (dict) .Values.podSecurityContext) \| default (dict) }}
		{{- $containerSecurityContext := merge (deepCopy (dig "container" $globalSecurityContexts (dict))) (default (dict) .Values.securityContext) \| default (dict) }}

Conversation

roderik commented Sep 18, 2025

Summary

Testing

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

github-actions Bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

seer-by-sentry Bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Sep 18, 2025 •

edited

Loading